pkg/planner/core/stats.go (1)

597-599: ⚠️ Potential issue | 🟠 Major

tiflashPath can still be appended as nil in heuristic keep-path logic.

The new hasTiFlashReplica guard at Line 599 protects path derivation, but the keep branch still appends tiflashPath without re-checking availability. When TiFlash is preferred/enforced but absent, this can inject a nil path.

🔧 Suggested fix

-		keep := (ds.PreferStoreType&h.PreferTiFlash != 0 || isMPPEnforced) && selected.StoreType != kv.TiFlash
+		keep := (ds.PreferStoreType&h.PreferTiFlash != 0 || isMPPEnforced) &&
+			hasTiFlashReplica && tiflashPath != nil && selected.StoreType != kv.TiFlash
 		if keep {
 			// also keep tiflash path as well.
 			ds.PossibleAccessPaths = append(ds.PossibleAccessPaths, tiflashPath)
 			return nil
 		}

Also applies to: 701-705

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/core/stats.go` around lines 597 - 599, The keep-path logic may
append tiflashPath even when hasTiFlashReplica is false, causing a nil path to
be injected; update the branches that append tiflashPath (the one gated by
(ds.PreferStoreType&h.PreferTiFlash != 0 || isMPPEnforced) and the similar block
around the 701–705 region) to only append tiflashPath when hasTiFlashReplica is
true (or tiflashPath != nil), i.e., add a guard checking hasTiFlashReplica (or
tiflashPath != nil) before pushing/appending tiflashPath in the keep-path branch
so the path is never nil. Ensure the same guard is applied consistently in both
places.

pkg/planner/core/find_best_task.go (1)

90-99: ⚠️ Potential issue | 🟠 Major

Keep ColumnNames aligned with ds.TblCols in the fallback branch.

If reconstruction fails, ds.OutputNames() can have different length/order than ds.TblCols, which can corrupt partition-pruning metadata carried in PhysPlanPartInfo.

Proposed fix

 func buildPhysPlanPartInfo(ds *logicalop.DataSource) *physicalop.PhysPlanPartInfo {
 	columnNames, err := rule.ReconstructTableColNames(ds)
 	if err != nil {
 		// Keep planning resilient. If reconstructing names fails, fall back to output
 		// names to avoid aborting optimization.
-		columnNames = ds.OutputNames()
+		fallback := ds.OutputNames()
+		if len(fallback) == len(ds.TblCols) {
+			columnNames = fallback
+		} else {
+			columnNames = make(types.NameSlice, len(ds.TblCols))
+			for i := range columnNames {
+				columnNames[i] = &types.FieldName{}
+			}
+		}
 	}
 	return &physicalop.PhysPlanPartInfo{
 		PruningConds:   ds.AllConds,
 		PartitionNames: ds.PartitionNames,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/core/find_best_task.go` around lines 90 - 99, The fallback branch
uses ds.OutputNames() which may not match ds.TblCols length/order and can
corrupt PhysPlanPartInfo.ColumnNames; change the err != nil branch in
find_best_task.go so columnNames is derived from ds.TblCols (e.g., map each
ds.TblCols entry to its name or otherwise construct a slice with the same
length/order as ds.TblCols) instead of using ds.OutputNames(), ensuring
ColumnNames aligns with ds.TblCols when returning the
physicalop.PhysPlanPartInfo.

pkg/planner/core/base/plan_base.go (1)

402-406: ⚠️ Potential issue | 🔴 Critical

Guard nil columns in PossiblePropertiesInfo.Hash64 to prevent panic.

Equals handles nil column slots, but Line 405 dereferences col unconditionally. A nil slot will panic during hashing.

Proposed fix

 		for _, one := range info.Orders {
 			h.HashInt(len(one))
 			for _, col := range one {
-				col.Hash64(h)
+				if col == nil {
+					h.HashByte(base.NilFlag)
+					continue
+				}
+				h.HashByte(base.NotNilFlag)
+				col.Hash64(h)
 			}
 		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/core/base/plan_base.go` around lines 402 - 406,
PossiblePropertiesInfo.Hash64 currently dereferences columns in info.Orders
without nil checks, which can panic; update the loop in
PossiblePropertiesInfo.Hash64 (iterating info.Orders and each col) to guard
against nil column slots by checking if col != nil before calling col.Hash64(h)
and otherwise incorporate a stable placeholder into the hash (e.g., call
h.HashInt(0) or another deterministic value) so nil and non-nil columns produce
distinct, consistent hashes.

pkg/planner/core/operator/physicalop/physical_hash_agg.go (1)

57-62: ⚠️ Potential issue | 🟠 Major

MPP-disabled sessions can still slip into MPP hash-agg enumeration.

The early return uses canPushDownToMPP before IsMPPAllowed() is applied. That allows MPP branches to survive in some prop.TaskTp == property.MppTaskType paths.

Proposed fix

-	var canPushDownToMPP bool
-	if util2.ShouldCheckTiFlashPushDown(la.SCtx(), logicalop.GetHasTiFlash(lp)) {
+	sessionVars := la.SCtx().GetSessionVars()
+	canPushDownToMPP := false
+	if sessionVars.IsMPPAllowed() &&
+		util2.ShouldCheckTiFlashPushDown(la.SCtx(), logicalop.GetHasTiFlash(lp)) {
 		canPushDownToMPP = checkCanPushDownToMPP(la)
 	}
 	if prop.TaskTp == property.MppTaskType && !canPushDownToMPP {
 		return nil
 	}
@@
-	canPushDownToMPP = canPushDownToMPP && la.SCtx().GetSessionVars().IsMPPAllowed()
+	// `canPushDownToMPP` already includes `IsMPPAllowed()`.
 	if canPushDownToMPP {
 		taskTypes = append(taskTypes, property.MppTaskType)
 	}

Also applies to: 71-73, 89-91

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/core/operator/physicalop/physical_hash_agg.go` around lines 57 -
62, The code uses canPushDownToMPP before checking session MPP permission,
letting MPP branches survive; ensure you first check IsMPPAllowed() on the
session context and only then evaluate util2.ShouldCheckTiFlashPushDown/ call
checkCanPushDownToMPP to set canPushDownToMPP. Concretely, wrap the existing
ShouldCheckTiFlashPushDown + checkCanPushDownToMPP logic behind an
IsMPPAllowed() guard (using la.SCtx() and IsMPPAllowed()), or set
canPushDownToMPP = false unless IsMPPAllowed() returns true, then compute it;
apply the same change to the other similar blocks around lines with prop.TaskTp
== property.MppTaskType (the occurrences using canPushDownToMPP).

pkg/planner/core/operator/logicalop/logical_union_all.go (1)

174-184: ⚠️ Potential issue | 🟡 Minor

HasTiflash incorrectly remains true when all children are nil.

The current logic on lines 175-180 starts with hasTiflash = true when children exist, then skips nil children. If all children are nil (e.g., [nil, nil]), hasTiflash remains true, which incorrectly enables MPP-capable propagation.

Suggested fix

 func (p *LogicalUnionAll) PreparePossibleProperties(_ *expression.Schema, childrenProperties ...*base.PossiblePropertiesInfo) *base.PossiblePropertiesInfo {
-	hasTiflash := len(childrenProperties) > 0
+	hasTiflash := false
+	hasValidChild := false
 	for _, child := range childrenProperties {
-		if child == nil {
-			continue
+		if child == nil || !child.HasTiflash {
+			hasTiflash = false
+			break
+		}
+		if !hasValidChild {
+			hasTiflash = true
+			hasValidChild = true
 		}
-		hasTiflash = hasTiflash && child.HasTiflash
 	}
 	p.hasTiflash = hasTiflash
 	return &base.PossiblePropertiesInfo{HasTiflash: p.hasTiflash}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/core/operator/logicalop/logical_union_all.go` around lines 174 -
184, The method PreparePossibleProperties in LogicalUnionAll sets hasTiflash
true when childrenProperties is non-empty even if all entries are nil; change
the logic so HasTiflash is true only when there is at least one non-nil child
and every non-nil child's HasTiflash is true. In practice, inside
LogicalUnionAll.PreparePossibleProperties introduce a seenNonNil (or similar)
boolean, set it when encountering a non-nil child while computing hasTiflash =
hasTiflash && child.HasTiflash, and after the loop set hasTiflash = false if
seenNonNil is false before assigning p.hasTiflash and returning the
PossiblePropertiesInfo. Ensure you reference the existing symbols:
LogicalUnionAll.PreparePossibleProperties, childrenProperties, p.hasTiflash, and
base.PossiblePropertiesInfo{HasTiflash: ...}.

pkg/planner/core/operator/logicalop/logical_window.go (1)

416-428: ⚠️ Potential issue | 🟠 Major

Guard against nil/empty infos before accessing infos[0].

Accessing infos[0].HasTiflash without checking len(infos) > 0 && infos[0] != nil will panic if this method is ever called with empty or nil child properties. While LogicalWindow typically has one child, defensive coding prevents planner panics.

🛡️ Proposed fix

 func (p *LogicalWindow) PreparePossibleProperties(_ *expression.Schema, infos ...*base.PossiblePropertiesInfo) *base.PossiblePropertiesInfo {
-	p.hasTiflash = infos[0].HasTiflash
+	hasTiflash := false
+	if len(infos) > 0 && infos[0] != nil {
+		hasTiflash = infos[0].HasTiflash
+	}
+	p.hasTiflash = hasTiflash
 	result := make([]*expression.Column, 0, len(p.PartitionBy)+len(p.OrderBy))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/core/operator/logicalop/logical_window.go` around lines 416 -
428, In PreparePossibleProperties of LogicalWindow, guard against infos being
empty or nil before reading infos[0]. Replace the unconditional read of
infos[0].HasTiflash with a check like if len(infos) > 0 && infos[0] != nil {
p.hasTiflash = infos[0].HasTiflash } else { p.hasTiflash = false } so
PreparePossibleProperties (and the Orders/HasTiflash returned) can't panic when
called with no child PossiblePropertiesInfo; keep building result from
PartitionBy/OrderBy unchanged and return the same base.PossiblePropertiesInfo
structure.

pkg/planner/cascades/old/optimize.go (1)

374-377: Normalize nil exprInfo to empty info for consistency with core path.

This is optional, but replacing continue with empty-info normalization keeps old-cascades behavior closer to pkg/planner/core/property_cols_prune.go and reduces divergent nil semantics.

♻️ Suggested change

-		exprInfo := expr.ExprNode.PreparePossibleProperties(expr.Schema(), childrenProperties...)
-		if exprInfo == nil {
-			continue
-		}
+		exprInfo := expr.ExprNode.PreparePossibleProperties(expr.Schema(), childrenProperties...)
+		if exprInfo == nil {
+			exprInfo = &base.PossiblePropertiesInfo{}
+		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/cascades/old/optimize.go` around lines 374 - 377, When
expr.ExprNode.PreparePossibleProperties(expr.Schema(), childrenProperties...)
returns nil, don't skip the node; instead normalize to an empty
possible-properties value consistent with the core implementation: replace the
`continue` with assigning an empty instance of the same possible-properties type
that PreparePossibleProperties returns (i.e., create/assign the zero/empty
possible-properties value) to `exprInfo` so subsequent logic expects a non-nil
info object; update uses of `exprInfo` in the surrounding code in optimize.go
accordingly.

pkg/planner/core/operator/logicalop/logical_projection.go (1)

334-335: Add a defensive guard before indexing childrenProperties[0].

This function currently assumes a non-empty, non-nil child info slice. A small guard would make it more robust against future caller changes.

🛡️ Suggested hardening

 func (p *LogicalProjection) PreparePossibleProperties(_ *expression.Schema, childrenProperties ...*base.PossiblePropertiesInfo) *base.PossiblePropertiesInfo {
+	if len(childrenProperties) == 0 || childrenProperties[0] == nil {
+		p.hasTiflash = false
+		return &base.PossiblePropertiesInfo{}
+	}
 	childProperties := childrenProperties[0]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/planner/core/operator/logicalop/logical_projection.go` around lines 334 -
335, Add a defensive guard at the start of
LogicalProjection.PreparePossibleProperties to avoid indexing into an empty or
nil childrenProperties slice: check if len(childrenProperties) == 0 or
childrenProperties[0] == nil and return nil (or an empty
*base.PossiblePropertiesInfo if the call-sites expect a non-nil pointer). This
touches the PreparePossibleProperties method and the childProperties variable
and ensures you don't dereference a missing child before using
base.PossiblePropertiesInfo.

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro